Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added package-module-winget and needed dependency powershell-execution-policy #101

Merged

Conversation

craigcomstock
Copy link
Contributor

Ticket: ENT-12127

@craigcomstock craigcomstock force-pushed the ENT-12144-package-module-winget branch from 03e5e20 to cb97135 Compare August 30, 2024 13:52
management/package-module-winget/README Outdated Show resolved Hide resolved
management/package-module-winget/README Outdated Show resolved Hide resolved
management/package-module-winget/README Outdated Show resolved Hide resolved
management/package-module-winget/README Outdated Show resolved Hide resolved
management/package-module-winget/package-module-winget.cf Outdated Show resolved Hide resolved
management/package-module-winget/package-module-winget.cf Outdated Show resolved Hide resolved
management/package-module-winget/winget-installed.cf Outdated Show resolved Hide resolved
cfbs.json Outdated Show resolved Hide resolved
@craigcomstock craigcomstock force-pushed the ENT-12144-package-module-winget branch 4 times, most recently from cacf3c4 to 5a35452 Compare September 4, 2024 16:32
Copy link
Member

@nickanderson nickanderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the module input hasn't yet been updated to target the data namespace.
Minor suggestion of style for README mentioning variables (use verbatim)

Overall seems to make sense to me.

management/package-method-winget/README.md Outdated Show resolved Hide resolved
cfbs.json Outdated Show resolved Hide resolved
@craigcomstock craigcomstock force-pushed the ENT-12144-package-module-winget branch 2 times, most recently from b14365d to 435c9e3 Compare September 4, 2024 18:50
cfbs.json Show resolved Hide resolved
Copy link
Member

@nickanderson nickanderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good enough to me (without running it). I would target data namspace explicitly with the module input.

And thinking about module input again, there must be some magic if it does default to the data namespace, because variables in def.json (where module input ultimately lands) default to the default namespace in the def bundle.

@nickanderson
Copy link
Member

Actually I just ran a test with cfbs, and it seems that namespace is required for input.

$ cfbs build
Error in cfbs.json for module 'lynis': The "namespace" field is required in module input elements
WARNING: At least one error encountered while validating your cfbs.json file.
Please see the error messages above and apply fixes accordingly.
If not fixed, these errors will cause your project to not build in future cfbs versions.

Modules:
001 masterfiles @ d5d5c61942a97ec5e3fd35478e590c5943db5ac5 (Downloaded)
002 lynis       @ 4db6d515c45873a9f3fa7ce78ece593027464b64 (Downloaded)

Steps:
001 masterfiles : run 'EXPLICIT_VERSION=3.24.0 EXPLICIT_RELEASE=1 ./prepare.sh -y'
001 masterfiles : copy './' 'masterfiles/'
002 lynis       : copy 'policy/main.cf' 'masterfiles/services/lynis/main.cf'
002 lynis       : json 'cfbs/def.json' 'masterfiles/def.json'
002 lynis       : input './input.json' 'masterfiles/def.json'

Generating tarball...

Build complete, ready to deploy 🐿
 -> Directory: out/masterfiles
 -> Tarball:   out/masterfiles.tgz

To install on this machine: sudo cfbs install
To deploy on remote hub(s): cf-remote deploy

snippet of my cfbs.json whre I removed the namespace key for the input step for setting tar_url.

      "input": [
        {
          "type": "string",
          "namespace": "lynis",
          "bundle": "globals",
          "variable": "version",
          "label": "Lynis version",
          "question": "What version of Lynis should be used?",
          "default": "3.1.1"
        },
        {
          "type": "string",
          "bundle": "globals",
          "variable": "tar_url",
          "label": "Tarball url",
          "question": "Where should clients download lynis from (http/https)",
          "default": "https://downloads.cisofy.com/lynis/lynis-$(version).tar.gz",
          "comment": "Some may want to self host the tarball within their own infrastructure. This provides an easy way to do that. Note, the archive should be named for the version of Lynis selected."
        },

Copy link
Member

@nickanderson nickanderson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you must set namespace for module input.

@craigcomstock
Copy link
Contributor Author

I didn't have to with cfbs 4.2.1 which is odd since we are both using that version. I suppose you are using a build index module and I am using a repo#commit. Maybe that's the diff.

@craigcomstock craigcomstock merged commit dc6e25e into cfengine:master Sep 4, 2024
@craigcomstock craigcomstock deleted the ENT-12144-package-module-winget branch September 4, 2024 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants